-
Notifications
You must be signed in to change notification settings - Fork 1
Remove the logic for skipping existing processing #735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #735 +/- ##
==========================================
+ Coverage 46.59% 46.80% +0.20%
==========================================
Files 91 91
Lines 9634 9664 +30
Branches 1262 1275 +13
==========================================
+ Hits 4489 4523 +34
+ Misses 4932 4928 -4
Partials 213 213 🚀 New features to boost your workflow:
|
| finalising: bool = False | ||
| dormant: bool = False | ||
| multigrid_watcher_active: bool = True | ||
| processing_enabled: bool = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm, by removing processing_enabled, are we moving towards relying entirely on the API calls from the backend server to determine whether the Analyser is activated for a given session?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The processing_enabled flag didn't really do what it says, the analyse one if the thing which determines whether the Analyser is activated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got you. So, the MultigridController will have no attribute related to whether the Analyser should be triggered, but these will be directly set when setting up the RSyncers via the messages sent from the backend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the decision on whether to analyse happens in the call to _start_rsyncer_multigrid. The MultigridController knows whether it does have analysers, but you're right it doesn't know if it should have analysers or not
tieneupin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my comments. The changes look good to me, so feel free to merge once you're happy with it, and I can have a look at building and testing the recent changes we've made.
When we had the TUI we introduced logic for skipping processing in the case that someone started a fresh murfey with existing data. This no longer makes any sense as murfey can handle switching between tomo and SPA without a fresh instance.
Therefore this PR removes all
skip_existing_processingandfirst_looplogic.The case that needs checking is if processing is disabled, then it should not become enabled on reconnection. From the code this loops ok to me as the
start_multigrid_watcherrequests to the instrument server readsession.processfrom the database.